Skip to content

feat(ci): add /assign GitHub Action with stale cleanup (Fixes #2630)#2634

Open
Ayush7614 wants to merge 5 commits into
vybestack:mainfrom
Ayush7614:feat/assign-github-action-2630
Open

feat(ci): add /assign GitHub Action with stale cleanup (Fixes #2630)#2634
Ayush7614 wants to merge 5 commits into
vybestack:mainfrom
Ayush7614:feat/assign-github-action-2630

Conversation

@Ayush7614

@Ayush7614 Ayush7614 commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Implements the documented /assign self-assignment workflow (assign.yml + assign-issue.sh): exact-match comment gate, eligibility (merged PR / prior assignment / trusted contributors / owner-member-collaborator), 3-issue cap, auto-assigned label, and sticky feedback when assignment succeeds or silently fails.
  • Adds scheduled stale cleanup (assign-stale-cleanup.yml + unassign-stale-issues.sh) to unassign auto-assigned issues with no linked PR activity for 14 days (exempts acoliver).
  • Updates CONTRIBUTING.md and adds YAML/script contract tests in scripts/tests/assign-workflow.test.js.

Fixes #2630.

Test plan

  • bash -n on both scripts
  • bunx vitest run --config scripts/tests/vitest.config.ts scripts/tests/assign-workflow.test.js (11/11 passed)
  • Comment /assign on an unassigned eligible issue and confirm assignment + sticky feedback + auto-assigned label
  • Comment /assign on an already-assigned issue and confirm refusal feedback
  • Manually run Assign Stale Cleanup via workflow_dispatch after labeling a stale test issue

Summary by CodeRabbit

  • New Features
    • Added exact-text self-assignment via /assign, with strict eligibility checks and an assignment cap.
    • Introduced deterministic “winner” election with automatic verification and verified rollback to prevent conflicting assignments.
    • Added daily stale cleanup that unassigns expired automation assignments after 14 days, preserving qualifying activity and exempting the maintainer.
  • Documentation
    • Expanded guidance for /assign, including eligibility, limits, feedback outcomes, and stale cleanup timing.
  • Tests
    • Added comprehensive workflow and behavioral coverage, including failure/rollback and cleanup edge cases.

@Ayush7614
Ayush7614 requested a review from acoliver as a code owner July 22, 2026 15:19
@coderabbitai

coderabbitai Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds /assign issue-comment automation with strict eligibility, caps, deterministic contention handling, durable history, and verified rollback. Adds daily stale-assignment cleanup based on timeline provenance and linked PR activity, workflow wiring, documentation, and extensive behavioral tests.

Changes

Issue Assignment Automation

Layer / File(s) Summary
Assignment eligibility, mutation, and contention handling
.github/scripts/assign-issue.sh, scripts/tests/assign-remediation*.test.js
Validates issue state and eligibility, applies targeted assignment mutations, elects a deterministic winner, verifies caps and labels, rolls back failed attempts, records history, and posts sticky feedback.
Stale assignment detection and removal
.github/scripts/unassign-stale-issues.sh, .github/workflows/assign-stale-cleanup.yml, scripts/tests/assign-workflow-behaviors.test.js
Discovers marked issues, evaluates timeline provenance and qualifying linked PRs, and safely removes stale automation assignments while preserving manual assignees and exemptions.
Workflow and durable history contract
.github/workflows/assign.yml, .github/scripts/record-assignment-history.sh, .github/assignment-history.txt, CONTRIBUTING.md
Configures exact /assign triggers, assignment-event history recording, scheduled cleanup, per-login history labels, repository guards, permissions, and documented behavior.
Fake GitHub testing infrastructure
scripts/tests/assign-helpers.js, scripts/tests/fake-gh.py, scripts/tests/*assign*.test.js
Adds a stateful fake GitHub API and integration suites covering pagination, malformed responses, races, rollback, provenance, workflow configuration, and cleanup behavior.

Estimated code review effort: 5 (Critical) | ~120 minutes

Suggested labels: ci/cd, Code Quality / Modularization

Suggested reviewers: acoliver

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description is missing the required TLDR, Dive Deeper, Reviewer Test Plan, Testing Matrix, and Linked issues / bugs sections from the template. Rewrite the PR description to follow the template and add the missing sections, including a linked issue reference and a filled testing matrix.
Docstring Coverage ⚠️ Warning Docstring coverage is 49.21% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: a new /assign GitHub Action plus stale cleanup.
Linked Issues check ✅ Passed The changes implement exact-match /assign handling, eligibility, and 14-day stale cleanup with the acoliver exception requested by #2630.
Out of Scope Changes check ✅ Passed The added workflows, scripts, tests, and docs all support the /assign and stale-cleanup objectives, with no clear unrelated changes.
📋 Issue Planner

Built with CodeRabbit's Coding Plans for faster development and fewer bugs.

View plan used: #2630

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

LLxprt PR Review – PR #2634

Issue Alignment

Implements the /assign workflow from #2630: exact-match /assign comment gate, eligibility via merged PR or prior assignment (backfill file, current assignments, or per-login history label), 3-issue cap, auto-assigned label, sticky bot feedback, and a daily stale cleanup that unassigns auto-assigned issues after 14 days with no linked PR activity while exempting acoliver. Evidence: .github/workflows/assign.yml, .github/workflows/assign-stale-cleanup.yml, .github/scripts/assign-issue.sh, .github/scripts/unassign-stale-issues.sh, .github/scripts/record-assignment-history.sh, and CONTRIBUTING.md updates.

Side Effects

  • Adds a scheduled workflow and 4 new scripts under .github/.
  • Creates per-user labels (asnhist--LOGIN) in the repository; bounded by actual assignees but could accumulate over time.
  • Uses github.token and issues: write; stale cleanup restricts scheduled runs to vybestack/llxprt-code.
  • record-assignment-history.sh runs on issues:assigned; bot-triggered assignments are also recorded directly from assign-issue.sh to avoid recursion gaps.

Code Quality

  • Strong fail-closed posture: pre/post mutation re-reads, targeted POST/DELETE mutations, verified rollback, and lifecycle traps.
  • Winner election uses timeline event position ordering and handles concurrent bot contenders; stale cleanup includes provenance validation, pre-delete revalidation, and co-assignee preservation.
  • Main risk is complexity in assign-issue.sh/unassign-stale-issues.sh; however, the scripts are well-structured with defensive checks and clear error paths.

Tests and Coverage

  • Coverage impact: increase.
  • Adds behavioral tests executing real bash scripts against a stateful fake gh adapter (scripts/tests/fake-gh.py), plus workflow YAML/contract tests.
  • Tests cover eligibility, cap, sticky feedback, exact-match gating, stale cleanup, pagination, linked PR detection, exemptions, and failure preservation.
  • Note: I could not execute tests in this environment (npm run test fails due to missing vitest/bun; PR author reports bunx vitest run --config scripts/tests/vitest.config.ts scripts/tests/assign-workflow.test.js passed 11/11).

Verdict

Ready — implementation matches issue requirements, side effects are bounded and documented, code quality is high with defensive concurrency handling, and test coverage materially increases with real-behavior assertions rather than mock theater.

Comment thread .github/scripts/unassign-stale-issues.sh Outdated
Comment thread .github/scripts/unassign-stale-issues.sh Outdated
Comment thread .github/scripts/assign-issue.sh Outdated
Comment thread .github/scripts/assign-issue.sh Outdated
Comment thread .github/workflows/assign-stale-cleanup.yml
Comment thread scripts/tests/assign-workflow.test.js
Comment thread scripts/tests/assign-workflow.test.js Outdated
Comment thread scripts/tests/assign-workflow.test.js
@github-actions

github-actions Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

OpenCodeReview — PR #2634

  • Reviewed head SHA: f4ebf79a11aa1190300f8252b71e3a804981a34c
  • Merge base: a20e30f14609ceca070fed5140510bbb36daee28
  • OCR version: open-code-review v1.7.9 (a32f852) linux/amd64 built at: 2026-07-14T03:41:57Z https://github.com/alibaba/open-code-review
  • Phase: review
  • Exit code: 0
  • Run: https://github.com/vybestack/llxprt-code/actions/runs/30075489627
  • 41 finding(s) (39 posted inline).
  • Artifacts: ocr-review-output contains raw JSON, stdout, stderr, preview, phase, and exit-code diagnostics.

Findings without a resolvable position

  • scripts/tests/assign-remediation5.test.js: > The comment says the side_effect fires on the 'pre-delete assignee GET', but the configured endpoint is 'repos/test/repo/issues/42' (an issue GET, not an assignee GET). Please update the comment to accurately describe the trigger so future maintainers are not misled about which API call drives the race.
  • scripts/tests/assign-remediation8.test.js: > The signal test uses a fixed 500-iteration × 10ms polling window (5s total). Under heavy CI load or slow filesystems, the hook file may not appear within this window, causing flaky failures. Consider either increasing the timeout or making it configurable via an environment variable (e.g., ASSIGN_SIGNAL_TIMEOUT_MS).

@acoliver

Copy link
Copy Markdown
Collaborator

@Ayush7614 run shellcheck

@acoliver

Copy link
Copy Markdown
Collaborator

@CodeRabbit is the shell necessary in here or could this be done entirely in yaml?

@coderabbitai

coderabbitai Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '== Candidate files =='
fd -a '^(assign\.yml|assign-issue\.sh|assign-stale-cleanup\.yml|unassign-stale-issues\.sh|assign-workflow\.test\.js)$' . \
  -E node_modules -E dist -E build

printf '%s\n' '== Workflow outlines =='
for f in $(fd -a '^(assign\.yml|assign-stale-cleanup\.yml)$' . -E node_modules -E dist -E build); do
  echo "--- $f"
  nl -ba "$f"
done

printf '%s\n' '== Script function outlines =='
for f in $(fd -a '^(assign-issue\.sh|unassign-stale-issues\.sh)$' . -E node_modules -E dist -E build); do
  echo "--- $f"
  rg -n '^(#!/|[[:alpha:]_][[:alnum:]_]*\(\)|[[:alpha:]_][[:alnum:]_]*\(\s*\{)|\b(gh|jq|curl)\b|case |if |while |for )' "$f" || true
done

Length of output: 984


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '== Repository change summary =='
git status --short
git diff --name-status HEAD~1..HEAD 2>/dev/null || true

printf '%s\n' '== /assign implementation references =='
rg -n -i -C 3 '/assign|auto-assigned|stale cleanup|unassign.*stale' \
  -g '*.yml' -g '*.yaml' -g '*.sh' -g 'assign-workflow.test.js' . \
  -g '!node_modules/**' -g '!dist/**' -g '!build/**' || true

printf '%s\n' '== Contract test =='
test_file="$(fd -a '^assign-workflow\.test\.js$' scripts/tests | head -n 1)"
if [ -n "$test_file" ]; then
  nl -ba "$test_file"
fi

Length of output: 8443


🏁 Script executed:

#!/bin/bash
set -euo pipefail

for f in \
  .github/workflows/assign.yml \
  .github/workflows/assign-stale-cleanup.yml \
  .github/scripts/assign-issue.sh \
  .github/scripts/unassign-stale-issues.sh
do
  if [ -f "$f" ]; then
    printf '\n%s\n' "===== $f ====="
    awk '{ printf "%4d  %s\n", NR, $0 }' "$f"
  else
    printf '%s\n' "MISSING: $f"
  fi
done

Length of output: 20136


@acoliver No—this cannot be done entirely in declarative GitHub Actions YAML.

YAML is appropriate for the triggers, exact-comment gate, permissions, concurrency, and scheduling. But the implementation also needs dynamic GitHub API work and control flow:

  • query merged PRs, prior assignments, and open-assignment counts;
  • find/update the sticky feedback comment;
  • verify that GitHub actually applied an assignment;
  • enumerate labeled issues and assignees;
  • inspect assignment timestamps and linked PR activity;
  • calculate the 14-day threshold, retry failed API calls, and loop over candidates.

Those require imperative code. The current checked-in shell scripts are a reasonable choice, particularly since the logic is independently contract-tested.

The alternative would be actions/github-script with embedded JavaScript/Octokit calls. That could remove the shell scripts (and likely checkout/chmod), but it would not be YAML-only—it would move the same complexity into inline JavaScript, which is generally less maintainable and harder to test than the present scripts.

@acoliver

Copy link
Copy Markdown
Collaborator

@CodeRabbit is there an attack vector here and if so can we limit it? thinking like 1000 bots smash us with /assign ?

@coderabbitai

coderabbitai Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

workflow="$(fd -a '^assign\.yml$' . -E node_modules -E dist -E build | head -n1)"
script="$(fd -a '^assign-issue\.sh$' . -E node_modules -E dist -E build | head -n1)"
test_file="$(fd -a '^assign-workflow\.test\.js$' . -E node_modules -E dist -E build | head -n1)"

printf '%s\n' '== Candidate paths =='
printf 'workflow=%s\nscript=%s\ntest=%s\n' "$workflow" "$script" "$test_file"

for file in "$workflow" "$script" "$test_file"; do
  if [ -n "$file" ] && [ -f "$file" ]; then
    printf '\n===== %s =====\n' "$file"
    nl -ba "$file"
  fi
done

Length of output: 1095


🏁 Script executed:

#!/bin/bash
set -euo pipefail

find . \
  -path '*/node_modules' -prune -o \
  -path '*/dist' -prune -o \
  -path '*/build' -prune -o \
  -type f \( \
    -name 'assign.yml' -o \
    -name 'assign-issue.sh' -o \
    -name 'assign-workflow.test.js' \
  \) -print |
while IFS= read -r file; do
  printf '\n===== %s =====\n' "$file"
  sed -n '=' "$file" | sed 'N;s/\n/\t/'
done

Length of output: 1959


🏁 Script executed:

#!/bin/bash
set -euo pipefail

for file in \
  .github/workflows/assign.yml \
  .github/scripts/assign-issue.sh \
  scripts/tests/assign-workflow.test.js
do
  if [ -f "$file" ]; then
    printf '\n===== %s =====\n' "$file"
    sed -n '1,260p' "$file"
  else
    printf 'MISSING: %s\n' "$file"
  fi
done

Length of output: 14287


@acoliver Yes. The permission boundary is reasonably narrow—an untrusted bot cannot gain repository write access or bypass GitHub’s assignee rules—but there is a workflow/API-consumption denial-of-service vector:

  • Every exact /assign issue comment starts a runner.
  • The script checks out the repository, performs several gh API/search requests, and may update the sticky feedback comment before rejecting an ineligible account.
  • The current concurrency key is per issue, so it coalesces spam on one issue, but 1,000 bots commenting on 1,000 issues can still create up to 1,000 jobs.

I would limit this in two layers:

  1. Repository-wide concurrency for this workflow, rather than per-issue concurrency. This creates a hard upper bound on active/pending /assign work and lets GitHub discard superseded queued spam. The tradeoff is that legitimate assignment requests may need to be reposted if a burst occurs.

    concurrency:
      group: ${{ github.workflow }}
      cancel-in-progress: false
  2. Fail closed as early as possible: add a job-level gate for repository-associated users where compatible with the intended policy, then keep the script’s authoritative eligibility checks. At minimum, put the cheap AUTHOR_ASSOCIATION check before the current assignment-count and search queries. Right now an unknown account consumes several API calls before it is denied.

Also, for failed eligibility, I would avoid posting/updating sticky feedback for every request—or only provide that feedback for known contributors. Otherwise attackers can deliberately generate persistent issue-comment noise and write API traffic.

The three-assignment cap protects issue ownership; it does not rate-limit invocation volume. I recommend treating global concurrency plus early rejection as required hardening before enabling this on a public repository.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/scripts/assign-issue.sh:
- Around line 95-137: Update the workflow permissions associated with the
assign-issue script to include pull-requests: read alongside the existing
permissions. Ensure the gh search prs invocation in the merged_pr_count
eligibility check can read pull requests and preserve the current eligibility
behavior.

In @.github/scripts/unassign-stale-issues.sh:
- Around line 148-166: Update the assignee evaluation in the function containing
the exemption loop so the `EXEMPT_LOGIN` check occurs inside the per-assignee
loop. Skip only `acoliver` while continuing to evaluate other co-assignees
through `assignment_age_ok_to_unassign`; remove the issue-level `return 0`
behavior and preserve the existing unassignment decision flow.

In @.github/workflows/assign.yml:
- Around line 11-13: Update the workflow permissions block in assign.yml to add
pull-requests: read alongside contents: read and issues: write, allowing the
merged pull-request search used by the workflow to function correctly.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 1f3d8a5f-64b7-4b6f-a790-aa65ed95e103

📥 Commits

Reviewing files that changed from the base of the PR and between 83bd764 and b89c0a9.

📒 Files selected for processing (6)
  • .github/scripts/assign-issue.sh
  • .github/scripts/unassign-stale-issues.sh
  • .github/workflows/assign-stale-cleanup.yml
  • .github/workflows/assign.yml
  • CONTRIBUTING.md
  • scripts/tests/assign-workflow.test.js

Comment thread .github/scripts/assign-issue.sh Outdated
Comment thread .github/scripts/unassign-stale-issues.sh Outdated
Comment thread .github/workflows/assign.yml
@acoliver

Copy link
Copy Markdown
Collaborator

@Ayush7614 look at #2634 (comment) and the other coderabbit reviews. LLet's assume at some point someone will comment /assign on everything so let's add a cap for how many issues they can be assigned at once, the concurrency cap. ask your agent for any other hardening recommendations to prevent also spammers (github name BuyMyStupidThing /assign every issue it can)

@Ayush7614

Copy link
Copy Markdown
Contributor Author

@acoliver addressed shellcheck + the spam-hardening notes (and CodeRabbit’s actionable items):

CI / shellcheck

  • Fixed SC2154 / SC2269 / SC2249 locally with shellcheck 0.11.0 (same flags as CI).

Spam / flood hardening

  • Hard 3 open-issue concurrency cap (already present; kept + documented more clearly).
  • Workflow now ignores Bot accounts.
  • Concurrency group is now per issue + actor, so a flood of /assign from one user cancels/queues on itself instead of fan-outing.
  • Eligibility still requires merged PR / prior assignment / trusted list / owner-member-collaborator before assign.
  • Non-collaborators still fail the post-assign verify and get sticky feedback (no silent “success”).

CodeRabbit

  • Added pull-requests: read so gh search prs --merged works under the explicit permissions block.
  • Stale cleanup now exempts only acoliver as an assignee (other co-assignees can still be unassigned).
  • Dropped the 50-PR list fallback; linked-PR detection uses search only.

Happy to add more rate-limiting (e.g. daily attempt budget) if you want that next.

Comment thread .github/scripts/assign-issue.sh Outdated
Comment thread .github/scripts/assign-issue.sh Outdated
Comment thread .github/scripts/unassign-stale-issues.sh Outdated
Comment thread .github/scripts/unassign-stale-issues.sh Outdated
Comment thread .github/scripts/unassign-stale-issues.sh Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
.github/scripts/assign-issue.sh (2)

87-133: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Do not grant self-assignment eligibility from repository roles or the trusted file.

The eligibility block accepts OWNER, MEMBER, COLLABORATOR, and .github/trusted-contributors.txt membership without a merged PR or prior issue assignment, while the failure feedback only lists those two conditions plus the trusted file. Remove the role bypass or document and test the role/trusted-file exceptions as intentional policy.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/scripts/assign-issue.sh around lines 87 - 133, Restrict
self-assignment eligibility in the eligibility logic around merged_pr_count and
prior_assigned_count to users with a merged PR or prior issue assignment. Remove
the TRUSTED_FILE membership check and the AUTHOR_ASSOCIATION role bypass,
including their eligibility reasons, so OWNER, MEMBER, COLLABORATOR, and
trusted-file entries cannot bypass those requirements.

137-169: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Make assignment and tracking-label updates a single failure domain.

If gh issue edit --add-label fails, or the run is canceled between the assignment and label write, the issue stays assigned without auto-assigned. The stale cleanup workflow selects open issues by label:auto-assigned, so this leave-behind assignment will not be discovered and may run against the per-user cap. Use one API update that sets the assignee and preserves existing labels, and report failure unless both states persist before printing success.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/scripts/assign-issue.sh around lines 137 - 169, The assignment and
auto-assigned label writes in the post-eligibility flow must be one atomic
tracking update. Replace the separate gh issue edit calls around the assignment
verification and label addition with a single API update that sets USER_LOGIN as
assignee while preserving all existing labels, then re-read the issue and report
failure unless both the assignee and AUTO_ASSIGNED_LABEL are present; only print
success after both states persist.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/scripts/assign-issue.sh:
- Around line 39-43: Update the existing_id lookup in the issue-assignment flow
to capture gh api output and its exit status separately, rather than masking
failures with || true. Fail closed when the API request returns non-zero, while
preserving the current behavior when the request succeeds but no comment matches
MARKER.
- Around line 40-42: Update the comment lookup in the assignment feedback flow
to match the marker only when the comment was authored by the configured bot
account, preventing user-authored marker copies from being patched. Preserve
pagination and first-match behavior, and use the existing bot identity
configuration rather than hardcoding an unrelated author.
- Around line 60-68: Serialize the entire assignment check-and-mutation flow in
the `/assign` workflow per issue, rather than per commenter, by updating the
concurrency group in the workflow configuration and setting cancel-in-progress
to false. Ensure the existing zero-assignee check and add-assignee operation in
the assignment script execute within that shared per-issue queue.
- Around line 60-63: Update the assignee count read in the assign guard to fail
closed: remove the fallback that converts gh issue view errors into 0, capture
or propagate the command failure, and abort with clear feedback when reading
current assignees fails. Continue to the assignment flow only when
assignee_count is successfully obtained.

---

Outside diff comments:
In @.github/scripts/assign-issue.sh:
- Around line 87-133: Restrict self-assignment eligibility in the eligibility
logic around merged_pr_count and prior_assigned_count to users with a merged PR
or prior issue assignment. Remove the TRUSTED_FILE membership check and the
AUTHOR_ASSOCIATION role bypass, including their eligibility reasons, so OWNER,
MEMBER, COLLABORATOR, and trusted-file entries cannot bypass those requirements.
- Around line 137-169: The assignment and auto-assigned label writes in the
post-eligibility flow must be one atomic tracking update. Replace the separate
gh issue edit calls around the assignment verification and label addition with a
single API update that sets USER_LOGIN as assignee while preserving all existing
labels, then re-read the issue and report failure unless both the assignee and
AUTO_ASSIGNED_LABEL are present; only print success after both states persist.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 472bf4a3-4bb3-470f-8835-e583148e1084

📥 Commits

Reviewing files that changed from the base of the PR and between b89c0a9 and 27cb386.

📒 Files selected for processing (6)
  • .github/scripts/assign-issue.sh
  • .github/scripts/unassign-stale-issues.sh
  • .github/workflows/assign-stale-cleanup.yml
  • .github/workflows/assign.yml
  • CONTRIBUTING.md
  • scripts/tests/assign-workflow.test.js
🚧 Files skipped from review as they are similar to previous changes (5)
  • .github/workflows/assign-stale-cleanup.yml
  • .github/workflows/assign.yml
  • CONTRIBUTING.md
  • scripts/tests/assign-workflow.test.js
  • .github/scripts/unassign-stale-issues.sh

Comment thread .github/scripts/assign-issue.sh Outdated
Comment thread .github/scripts/assign-issue.sh Outdated
Comment thread .github/scripts/assign-issue.sh Outdated
Comment thread .github/scripts/assign-issue.sh Outdated
Ayush7614 and others added 3 commits July 23, 2026 22:13
Implement the documented self-assignment slash command and a scheduled
cleanup for auto-assigned issues with no PR activity for two weeks.
Fixes vybestack#2630.
Satisfy shellcheck, add pull-requests:read, reject Bot actors, serialize
per-actor concurrency, keep the 3-issue cap, and unassign stale
co-assignees without skipping whole issues for acoliver.
@acoliver
acoliver force-pushed the feat/assign-github-action-2630 branch from 27cb386 to 0204988 Compare July 24, 2026 04:52
@acoliver acoliver changed the title feat(ci): add /assign GitHub Action with stale cleanup feat(ci): add /assign GitHub Action with stale cleanup (Fixes #2630) Jul 24, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
scripts/tests/assign-remediation3.test.js (1)

48-82: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

makeLabeledEvent and makeUnlabeledEvent are re-implemented here but already exist in assign-helpers.js. Importing them would avoid drift between the two definitions.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/tests/assign-remediation3.test.js` around lines 48 - 82, Remove the
duplicate makeLabeledEvent and makeUnlabeledEvent definitions from this test and
import both helpers from assign-helpers.js. Update any references as needed
while preserving the existing test behavior and arguments.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/scripts/assign-issue.sh:
- Around line 392-405: Update verified_rollback_and_fail and its non-contention
callers so feedback_msg is posted after a successful rollback for API-failure
and mutation-cap paths, preserving the existing exit codes. Keep successful
contention-loser rollbacks silent to avoid overwriting the winner’s sticky
feedback, while retaining the current rollback-failure handling.

---

Nitpick comments:
In `@scripts/tests/assign-remediation3.test.js`:
- Around line 48-82: Remove the duplicate makeLabeledEvent and
makeUnlabeledEvent definitions from this test and import both helpers from
assign-helpers.js. Update any references as needed while preserving the existing
test behavior and arguments.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 32eeab8c-2569-420f-844e-9508436186dd

📥 Commits

Reviewing files that changed from the base of the PR and between 27cb386 and c71fe4a.

📒 Files selected for processing (18)
  • .github/assignment-history.txt
  • .github/scripts/assign-issue.sh
  • .github/scripts/record-assignment-history.sh
  • .github/scripts/unassign-stale-issues.sh
  • .github/workflows/assign-stale-cleanup.yml
  • .github/workflows/assign.yml
  • CONTRIBUTING.md
  • scripts/tests/assign-helpers.js
  • scripts/tests/assign-remediation.test.js
  • scripts/tests/assign-remediation2.test.js
  • scripts/tests/assign-remediation3.test.js
  • scripts/tests/assign-remediation4.test.js
  • scripts/tests/assign-remediation5.test.js
  • scripts/tests/assign-remediation6.test.js
  • scripts/tests/assign-remediation7.test.js
  • scripts/tests/assign-workflow-behaviors.test.js
  • scripts/tests/assign-workflow.test.js
  • scripts/tests/fake-gh.py
🚧 Files skipped from review as they are similar to previous changes (2)
  • .github/workflows/assign-stale-cleanup.yml
  • CONTRIBUTING.md

Comment thread .github/scripts/assign-issue.sh
Comment thread .github/scripts/record-assignment-history.sh
Comment thread .github/scripts/record-assignment-history.sh
Comment thread .github/scripts/record-assignment-history.sh Outdated
Comment thread .github/scripts/record-assignment-history.sh
Comment thread .github/scripts/record-assignment-history.sh Outdated
Comment thread scripts/tests/assign-workflow.test.js Outdated
Comment thread scripts/tests/assign-workflow.test.js
Comment thread scripts/tests/assign-workflow.test.js
Comment thread scripts/tests/assign-workflow.test.js
Comment thread scripts/tests/fake-gh.py
Comment on lines +17 to +22
validate_github_login() {
local login="$1"
[[ -n "${login}" ]] || return 1
[[ "${#login}" -le 39 ]] || return 1
[[ "${login}" =~ ^[A-Za-z0-9]([A-Za-z0-9-]*[A-Za-z0-9])?$ ]] || return 1
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The regex in validate_github_login allows consecutive hyphens (e.g., user--name). GitHub usernames require each hyphen to be surrounded by alphanumeric characters. Consider updating the pattern to ^[A-Za-z0-9]+(-[A-Za-z0-9]+)*$, which enforces non-empty segments separated by single hyphens and naturally prevents leading, trailing, or consecutive hyphens.

Comment on lines +17 to +22
validate_github_login() {
local login="$1"
[[ -n "${login}" ]] || return 1
[[ "${#login}" -le 39 ]] || return 1
[[ "${login}" =~ ^[A-Za-z0-9]([A-Za-z0-9-]*[A-Za-z0-9])?$ ]] || return 1
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No test coverage was added for the new validate_github_login function. This validation logic contains subtle boundary conditions (empty input, max length, single-character names, leading/trailing/consecutive hyphens) that should be covered by unit tests to prevent regressions.

Comment on lines +115 to +118
1)
printf '‼ Label %q exists with conflicting definition\n' "${LABEL_NAME}" >&2
exit 1
;;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Misleading error message when POST fails: validate_history_label conflates 'absent' and 'collision' by returning 1 for both cases. If the POST fails for transient reasons (network error, rate limit, permissions) and the label is still absent, this script incorrectly reports Label %q exists with conflicting definition instead of indicating the creation actually failed. Consider splitting the return codes (e.g., return 2 for collision, matching validate_label_definition in assign-issue.sh) so the post-POST check can distinguish these cases.

Comment on lines +101 to +105
if ! gh api --method POST "repos/${REPO}/labels" \
-f name="${LABEL_NAME}" \
-f color="${HISTORY_COLOR}" \
-f description="${HISTORY_DESC}" \
--silent >/dev/null 2>&1; then

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The POST request to create the label uses --silent >/dev/null 2>&1, which completely discards the actual GitHub API error message. If creation fails for reasons other than concurrent creation (e.g., invalid color format, insufficient permissions, rate limiting), the script only reports a generic failure without the underlying cause, making debugging difficult. Consider capturing stderr on failure to include the actual API error in the diagnostic output.


# Source shared constants (history-label policy values + login validation).
# shellcheck source-path=SCRIPTDIR
source "$(dirname "${BASH_SOURCE[0]}")/assign-constants.sh"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The script does not validate that required constants (HISTORY_PREFIX, HISTORY_COLOR, HISTORY_DESC) from assign-constants.sh are set and valid before use. If any constant is missing or empty, the script will construct an invalid label name or send an invalid API request, leading to unpredictable failures. Add explicit checks (e.g., : "${HISTORY_PREFIX:?Missing HISTORY_PREFIX}") after sourcing the constants file.

Comment thread scripts/tests/fake-gh.py
Comment on lines +961 to +964
validate_array_field(body, "assignees")
validate_array_field(body, "labels")
if "assignees" in body:
issue["_assignees"] = list(body["assignees"])

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

handle_patch for /issues/{number} does not filter out unassignable logins, unlike handle_post for /issues/{number}/assignees which silently drops them. The module docstring documents "silent dropping of configured unassignable logins", and the POST handler implements this at lines 877-880, but the PATCH handler assigns body["assignees"] directly at line 964 without this check. This inconsistency means PATCH requests can add unassignable users to issues, breaking tests that rely on unassignable login behavior and diverging from the documented behavior.

Comment thread scripts/tests/fake-gh.py
Comment on lines +644 to +654
# GitHub defaults omitted state to "open" for /issues listing.
state_filter = params.get("state", "open")
assignee_filter = params.get("assignee")
label_filter = params.get("labels")
for _num_str, issue in state.get("issues", {}).items():
if state_filter == "open" and issue.get("state", "open") != "open":
continue
if state_filter == "closed" and issue.get("state") != "closed":
continue
if state_filter == "all":
pass

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Invalid state filter values in both /issues listing and search/issues are silently accepted and fall through without filtering, causing all items to be returned regardless of their actual state. For example, state=invalid or state:invalid bypasses the open/closed filtering logic entirely because only open, closed, and all are recognized. This could lead to incorrect test assertions when malformed or unexpected state parameters are passed.

Comment thread scripts/tests/fake-gh.py
Comment on lines +809 to +813
# Final type filter
if item_type == "pr":
results = [r for r in results if r.get("pull_request") is not None]
elif item_type == "issue":
results = [r for r in results if r.get("pull_request") is None]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Invalid type: or is: qualifier values in search/issues (e.g. type:invalid, is:invalid) are silently ignored, causing both issues and PRs to be returned instead of rejecting the query or returning empty results. For type:invalid, the final type filter at lines 810-813 does not match, so both issues and PRs are included. For is:invalid, the unknown qualifier is silently dropped, which may mask bugs in search query construction in the scripts under test.

Comment thread scripts/tests/fake-gh.py
Comment on lines +846 to +851
if body and isinstance(body, list):
label_names = body
elif body and "labels" in body and isinstance(body["labels"], list):
label_names = body["labels"]
else:
label_names = []

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In handle_post for /issues/{number}/labels (lines 846-851) and /issues/{number}/assignees (lines 871-876), if the request body contains a labels or assignees field that is not a list (e.g. a string), the fake silently falls back to an empty list instead of returning a validation error. This is inconsistent with validate_array_field, which exists to reject non-list array fields, and with the documented behavior that string-typed array fields are rejected. Malformed request bodies should be rejected rather than silently ignored to avoid masking bugs in the scripts under test.

Comment thread scripts/tests/fake-gh.py
Comment on lines +871 to +876
if body and "assignees" in body and isinstance(body["assignees"], list):
logins = body["assignees"]
elif body and isinstance(body, list):
logins = body
else:
logins = []

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In handle_post for /issues/{number}/assignees (lines 871-876), if the request body contains an assignees field that is not a list (e.g. a string), the fake silently falls back to an empty list instead of returning a validation error. This is inconsistent with validate_array_field, which exists to reject non-list array fields, and with the documented behavior that string-typed array fields are rejected. Malformed request bodies should be rejected rather than silently ignored to avoid masking bugs in the scripts under test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

github action of /assign

2 participants